-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[feat aga] Implement endpoint management for endpoint groups in accelerator #4471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[feat aga] Implement endpoint management for endpoint groups in accelerator #4471
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shraddhabang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
328d317 to
557f5e3
Compare
557f5e3 to
06b6baa
Compare
| // requeueReasonEndpointsInWarningState indicates that the reconciliation is being requeued because | ||
| // there are endpoints in warning state that need to be periodically rechecked | ||
| requeueReasonEndpointsInWarningState = "Retrying endpoints for Global Accelerator %s which did load successfully - will check availability again soon" | ||
| statusUpdateRequeueTime = 1 * time.Minute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: Just thinking, should we have different statusUpdateRequeueTime for requeueReasonAcceleratorInProgress and requeueReasonEndpointsInWarningState. would Endpoint issue (service is not ready, DNS is not ready) take more longer to resolve ?
|
|
||
| // buildEndpointConfiguration creates an EndpointConfiguration from a GlobalAcceleratorEndpoint | ||
| // This helper function consolidates the repeated code for creating endpoint configurations | ||
| func buildEndpointConfigurationFromEndpoint(endpoint *agaapi.GlobalAcceleratorEndpoint) agamodel.EndpointConfiguration { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildEndpointConfigurationFromEndpoint is not used .
| } | ||
| } | ||
|
|
||
| // Now remove endpoints that are no longer needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the way you how you handle add/removing endpoints. I feel we could make it even better to avoid zero downtime. Maybe even in the Flip-Flop approach. We can consider mot removing all endpoints at once. something like batched remove and add. But I am also thinking if only few customer will hit into this case, we can use what you have now. We don't want to over engineer it.
This PR implements optimized endpoint management for AWS Global Accelerator resources with intelligent handling of endpoint states and API interactions.
Commit 1: [feat aga] Implement endpoints builder filtering for partially loaded…
This change ensures that warning state endpoints don't block the successful deployment of healthy endpoints. While we want to eventually deploy these endpoints when they become healthy, we don't want them to prevent the deployment of already-healthy endpoints.
Commit 2: [feat aga] Implement endpoints deployer with requeue logic for partia…
Implemented comprehensive endpoint drift detection:
Optimized API usage pattern:
Added intelligent error handling with flip-flop pattern:
Implemented warning state requeuing:
Added comprehensive unit tests for all new functionality
Checklist
README.md, or thedocsdirectory)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯